Conversation
|
Caution Review failedThe pull request is closed. WalkthroughTwo clap argument configurations in the moq-native crate were changed to require equals-syntax parsing. The 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rs/moq-native/src/client.rs (1)
355-358:⚠️ Potential issue | 🔴 CriticalTest
test_cli_disable_verify_explicit_falsewill fail withrequire_equals = true.With
require_equals = true, space-separated values like--tls-disable-verify falseare no longer accepted —falsewill be treated as an unrecognized positional argument. The test must use the equals form.🐛 Proposed fix
#[test] fn test_cli_disable_verify_explicit_false() { - let config = ClientConfig::parse_from(["test", "--tls-disable-verify", "false"]); + let config = ClientConfig::parse_from(["test", "--tls-disable-verify=false"]); assert_eq!(config.tls.disable_verify, Some(false)); }
🧹 Nitpick comments (1)
rs/moq-native/src/client.rs (1)
349-352: Consider also updating thetest_cli_disable_verify_flagtest for consistency.While
--tls-disable-verify(bare flag) still works due tonum_args = 0..=1+default_missing_value, adding the equals form here too would make the test suite consistently demonstrate the new required syntax.
1 participant
No description provided.